Reland "[HIPSPV] Add in-tree SPIR-V backend support for chipStar" - #213052
Conversation
|
@llvm/pr-subscribers-backend-spir-v @llvm/pr-subscribers-clang Author: Paulius Velesko (pvelesko) ChangesFixes the buildbot breakage from #206910 on builders that do not build the SPIR-V target (e.g. llvm-clang-x86_64-sie-ubuntu-fast). IsIntegratedBackendDefault() was made to depend on whether the in-tree SPIR-V backend is registered. That default also controls whether clang collapses the compile and backend jobs into a single -cc1 invocation, so without the SPIR-V target every HIPSPV RDC device compile was split into an -emit-llvm-bc job plus a separate -emit-obj job, failing Clang :: Driver/hipspv-toolchain-rdc.hip. This keeps the base class default and decides the llvm-spirv translator fallback in HIPSPV::Linker::constructLinkAndEmitSpirvCommand. IsIntegratedBackendSupported() still reports whether the backend is built, so -fintegrated-objemitter is still diagnosed when it is not, and -fno-integrated-objemitter still selects the translator. Verified both ways: with the SPIR-V target built, and with backend availability forced off to emulate the bot, where hipspv-toolchain-rdc.hip now passes. Full diff: https://github.com/llvm/llvm-project/pull/213052.diff 2 Files Affected:
diff --git a/clang/lib/Driver/ToolChains/HIPSPV.cpp b/clang/lib/Driver/ToolChains/HIPSPV.cpp
index e67b1541ef51b..6c99ff54adcb7 100644
--- a/clang/lib/Driver/ToolChains/HIPSPV.cpp
+++ b/clang/lib/Driver/ToolChains/HIPSPV.cpp
@@ -49,6 +49,12 @@ static std::string findPassPlugin(const Driver &D,
return std::string();
}
+// Is the in-tree SPIR-V backend built into this clang?
+static bool isSPIRVBackendAvailable(const llvm::Triple &T) {
+ std::string IgnoredError;
+ return llvm::TargetRegistry::lookupTarget(T, IgnoredError);
+}
+
// Runs the HipSpvPasses plugin via `opt` on TempFile when the plugin is found.
// Returns the lowered bitcode path, or TempFile unchanged if no plugin exists.
static const char *runHipSpvPasses(Compilation &C, const JobAction &JA,
@@ -108,7 +114,9 @@ void HIPSPV::Linker::constructLinkAndEmitSpirvCommand(
TempFile = runHipSpvPasses(C, JA, *this, getToolChain(), Inputs, Output,
Args, Name, TempFile);
- if (!getToolChain().useIntegratedBackend()) {
+ // Note that useIntegratedBackend() is consulted first so that an explicit
+ // -f(no-)integrated-objemitter still gets diagnosed against this toolchain.
+ if (!getToolChain().useIntegratedBackend() || !isSPIRVBackendAvailable(T)) {
// External translator path: BC -> SPIR-V via llvm-spirv.
llvm::opt::ArgStringList TrArgs;
if (T.getSubArch() == llvm::Triple::NoSubArch)
@@ -254,15 +262,8 @@ HIPSPVToolChain::HIPSPVToolChain(const Driver &D, const llvm::Triple &Triple,
}
bool HIPSPVToolChain::IsIntegratedBackendSupported() const {
- // The in-tree SPIR-V backend can only be used when it is built.
- std::string IgnoredError;
- return llvm::TargetRegistry::lookupTarget(getTriple(), IgnoredError);
-}
-
-bool HIPSPVToolChain::IsIntegratedBackendDefault() const {
- // Prefer the in-tree SPIR-V backend; fall back to the external llvm-spirv
- // translator when the backend is not built.
- return IsIntegratedBackendSupported();
+ // The in-tree SPIR-V backend can only be requested when it is built.
+ return isSPIRVBackendAvailable(getTriple());
}
void HIPSPVToolChain::addClangTargetOptions(
diff --git a/clang/lib/Driver/ToolChains/HIPSPV.h b/clang/lib/Driver/ToolChains/HIPSPV.h
index 176ef5ddde8b3..e38a7135a60fd 100644
--- a/clang/lib/Driver/ToolChains/HIPSPV.h
+++ b/clang/lib/Driver/ToolChains/HIPSPV.h
@@ -55,7 +55,12 @@ class LLVM_LIBRARY_VISIBILITY HIPSPVToolChain final : public ToolChain {
return HostTC ? &HostTC->getTriple() : nullptr;
}
- bool IsIntegratedBackendDefault() const override;
+ // Keep IsIntegratedBackendDefault() at the base class' "true": it also
+ // decides whether clang's compile and backend jobs are collapsed into a
+ // single -cc1 invocation, so making it depend on whether the SPIR-V backend
+ // was built would change the device compilation job layout of every HIPSPV
+ // compile. The fallback to the external llvm-spirv translator is decided in
+ // HIPSPV::Linker::constructLinkAndEmitSpirvCommand instead.
bool IsIntegratedBackendSupported() const override;
bool IsNonIntegratedBackendSupported() const override { return true; }
|
|
Can you either land this or revert your original change, to unbreak HEAD? |
|
@pvelesko Since #206910 was reverted, could you update this PR to reland the original change together with this fix? The normal pre-merge CI does not test a build without the SPIR-V target, and most HIPSPV tests are skipped in that configuration. Please also test both RDC and non-RDC chipStar flows with the SPIR-V target disabled, including actually running the llvm-spirv fallback. I can review the combined change and results. |
…vm#206910) This reverts commit 6162922, which reverted llvm#206910. The original change is relanded here unmodified; the buildbot breakage that caused the revert is fixed by the next commit in this PR.
…ckend llvm#206910 made HIPSPVToolChain::IsIntegratedBackendDefault() return false when the in-tree SPIR-V backend is not built. That default does more than select the SPIR-V emitter: it also decides whether clang collapses the compile and backend jobs into one -cc1 invocation. On builds without the SPIR-V target every HIPSPV RDC device compile therefore got split into an -emit-llvm-bc job plus a separate -emit-obj job, which broke Clang :: Driver/hipspv-toolchain-rdc.hip on X86-only bots such as llvm-clang-x86_64-sie-ubuntu-fast. Keep the base class default (integrated) and decide the llvm-spirv translator fallback where it belongs, in HIPSPV::Linker::constructLinkAndEmitSpirvCommand. IsIntegratedBackendSupported() still reports whether the backend is built, so an explicit -fintegrated-objemitter is still diagnosed when it is not, and -fno-integrated-objemitter still selects the translator. The device compilation job layout is now independent of which LLVM targets are built, so hipspv-toolchain-rdc.hip passes in both configurations.
The existing HIPSPV driver tests are guarded by the spirv-registered-target feature, so a builder configured without the SPIR-V target (e.g. an X86-only bot) skips nearly all of them. That is how the job-layout regression fixed in the previous commit reached the buildbots. Add a test that is deliberately not guarded: it pins the RDC device compilation job layout, which is a property of the driver rather than of the registered targets, and checks that -fno-integrated-objemitter selects the external llvm-spirv translator. Both hold in either configuration.
33e0515 to
3ca7d4f
Compare
|
@yxsamliu Updated: commit 1 relands #206910 unmodified, commit 2 is the fix, commit 3 adds a test not guarded by spirv-registered-target. Tested in X86;SPIRV and X86 only builds. Driver tests pass in both. Non-RDC and RDC (new driver, cross-TU) flows were run for real: the X86 only build emits through llvm-spirv, the X86;SPIRV build through the in-tree backend, both spirv-val clean. RDC with the old offload driver fails in both configurations, but reproduces with the pre-#206910 sources on the same LLVM, so it is pre-existing on main. |
|
@yxsamliu please merge when you can, I have no write access. |
|
@yxsamliu is it possible to back port this to LLVM 23? |
…vm#213052) Relands llvm#206910 (reverted in llvm#213088) with the fix for the breakage. IsIntegratedBackendDefault() was tied to whether the SPIR-V backend is registered, but it also controls whether clang collapses the compile and backend jobs, so builds without the SPIR-V target split every HIPSPV RDC device compile and failed hipspv-toolchain-rdc.hip. The default is back to the base class value and the translator fallback is decided in constructLinkAndEmitSpirvCommand. Third commit adds a test not guarded by spirv-registered-target. Validated in X86;SPIRV and X86 only builds: driver tests pass in both, and non-RDC plus RDC (new driver) flows run for real emit spirv-val clean modules via the backend and via llvm-spirv respectively.
…vm#213052) Relands llvm#206910 (reverted in llvm#213088) with the fix for the breakage. IsIntegratedBackendDefault() was tied to whether the SPIR-V backend is registered, but it also controls whether clang collapses the compile and backend jobs, so builds without the SPIR-V target split every HIPSPV RDC device compile and failed hipspv-toolchain-rdc.hip. The default is back to the base class value and the translator fallback is decided in constructLinkAndEmitSpirvCommand. Third commit adds a test not guarded by spirv-registered-target. Validated in X86;SPIRV and X86 only builds: driver tests pass in both, and non-RDC plus RDC (new driver) flows run for real emit spirv-val clean modules via the backend and via llvm-spirv respectively.
LLVM 23 is pinned to the upstream tag llvmorg-23.1.0-rc2 plus two patches; all four patches were verified to apply with git apply against a pristine rc2 checkout and llvm_release_230, and the patched HIPSPV.cpp and SPIRVSubtarget.cpp compile. 0001 backports llvm/llvm-project#213052 (merged 2026-07-31 as 7ef0ca2b13f9, a reland of #206910), which landed after release/23.x was cut. It makes the HIPSPV toolchain emit device SPIR-V with the in-tree SPIR-V backend by default and fall back to llvm-spirv under -fno-integrated-objemitter or when the SPIR-V target was not built. The upstream commit sits on a main that already had the extension list and the debug-info translator flags; release/23.x has neither, so both are folded in and a patched 23 matches main exactly. 0002 honors -g for device code, but only when the in-tree backend is the effective emitter. The translator encodes DebugTypeComposite with a Parent operand, producing a cyclic forward reference that spirv-val rejects and IGC mis-handles, so -g must keep being stripped there. The remaining llvm-21/llvm-22 patches are unnecessary here: the SPIR-V version and extension selection, static device library unbundling, the data layout and the macOS Mach-O support are all already in 23. Refs: #1004
…vm#213052) Relands llvm#206910 (reverted in llvm#213088) with the fix for the breakage. IsIntegratedBackendDefault() was tied to whether the SPIR-V backend is registered, but it also controls whether clang collapses the compile and backend jobs, so builds without the SPIR-V target split every HIPSPV RDC device compile and failed hipspv-toolchain-rdc.hip. The default is back to the base class value and the translator fallback is decided in constructLinkAndEmitSpirvCommand. Third commit adds a test not guarded by spirv-registered-target. Validated in X86;SPIRV and X86 only builds: driver tests pass in both, and non-RDC plus RDC (new driver) flows run for real emit spirv-val clean modules via the backend and via llvm-spirv respectively.
|
/cherry-pick 7ef0ca2 |
|
Failed to cherry-pick: 7ef0ca2 https://github.com/llvm/llvm-project/actions/runs/31112381662 Please manually backport the fix and push it to your github fork. Once this is done, please create a pull request |
|
This also fixes #208711 |
…vm#213052) Relands llvm#206910 (reverted in llvm#213088) with the fix for the breakage. IsIntegratedBackendDefault() was tied to whether the SPIR-V backend is registered, but it also controls whether clang collapses the compile and backend jobs, so builds without the SPIR-V target split every HIPSPV RDC device compile and failed hipspv-toolchain-rdc.hip. The default is back to the base class value and the translator fallback is decided in constructLinkAndEmitSpirvCommand. Third commit adds a test not guarded by spirv-registered-target. Validated in X86;SPIRV and X86 only builds: driver tests pass in both, and non-RDC plus RDC (new driver) flows run for real emit spirv-val clean modules via the backend and via llvm-spirv respectively. (cherry picked from commit 7ef0ca2)
Relands #206910 (reverted in #213088) with the fix for the breakage.
IsIntegratedBackendDefault() was tied to whether the SPIR-V backend is
registered, but it also controls whether clang collapses the compile and backend
jobs, so builds without the SPIR-V target split every HIPSPV RDC device compile
and failed hipspv-toolchain-rdc.hip. The default is back to the base class value
and the translator fallback is decided in constructLinkAndEmitSpirvCommand.
Third commit adds a test not guarded by spirv-registered-target.
Validated in X86;SPIRV and X86 only builds: driver tests pass in both, and
non-RDC plus RDC (new driver) flows run for real emit spirv-val clean modules
via the backend and via llvm-spirv respectively.